Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark as Hot Clip from live view #389

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

evilC
Copy link
Contributor

@evilC evilC commented Jan 24, 2024

Fixes #341
This feature allows the user to set a shortcut via the Input menu (Called "Mark Hot Clip")
image

When activated, it will:

  1. Stop recording if currently recording
  2. Mark the last clip as a Hot Clip
  3. Restart recording if we were originally recording

@evilC
Copy link
Contributor Author

evilC commented Jan 24, 2024

Please be aware that I am a total noob at C, so this is quite possibly shit code.
I had to make walk_sdcard() and mark_video_file() in ui/page_playback.c not static, as I could not work out how to reference it from my code in dvr.c otherwise - is there a better way of doing this? Did they need to be static in the first place?
Some additional thoughts:
I only need to call walk_sdcard to know if there are any clips or not, and mark_video_file is always passed 0 (I assume that the last clip will always be seq of 0, it seems to be in my testing) - could I maybe just blindly call mark_video_file(0)? Could this cause a crash? (eg if there were no clips, or SD not initialized?) Could it be made robust?

@Master92
Copy link
Contributor

Good job figuring that out as your first pull request, even though you claim to be "a total noob at C".

Personally, I'd implement it differently as those two methods from page_playback are actually meant to be only used in connection with its media database. walk_sdcard() could potentially become really slow over time and should really not be used during live video imho. It even copies all .jpg files to /tmp as a side effect.
Although it should work for now, it will definitely break as soon as date-folders are introduced (I'm currently working on that).

If you ask me, you should better query the recorder app for the current file name and continue renaming this file (and its thumbnail) with that information. I know that this is a bit more complicated but that's the only safe way I can think of getting the actual current dvr filename (at least for now).

@evilC
Copy link
Contributor Author

evilC commented Jan 25, 2024

Yeah, I felt kind of dirty doing it the way I did, I suspected as much.
TBH, thinking about it, I was approaching it from the use-case that I would use it in - I would always be landed when triggering it, so had not really thought of live view being "active" per se (Yes, the feed would be live for me, but I would not be airborne, so no real risk if it chugged the system a bit).

WRT querying the recorder app, iI had a look and it was pretty incomprehensible to me what is going on (Or maybe I am looking in the wrong place?). Furthermore, doing it like that would only work if we are currently recording, and not if we had stopped recording.
One idea I had was that maybe the recorder should write some variable somewhere which stores the name of the last recorded clip. This of course would mean that the shortcut would not work if you booted up and then hit it, but I don't really see a need to support that - certainly for me, the two use-cases are (1) landed, quad still powered up, and want to hot clip, and (2) landed, unplugged, and want to hot clip.

Another idea I had was to keep basically the current implementation, but avoid calling walk_sdcard. I have implemented this for now as it was pretty simple to do.
I have not yet tested, however, the scenario of what would happen if there were no files on the SD card.
[Edit] - aww tits, I realized of course that the media db is not initialized without calling walk_sdcard, so this method is not gonna work.

@evilC
Copy link
Contributor Author

evilC commented Jan 26, 2024

If you ask me, you should better query the recorder app for the current file name

I had a look at this and I could not see how to get this information :(
I see here that sFile probably holds the filename, but I tried logging it out using LOGI and don't see it appear in the logs. I notice that all the other log statements in the file are LOGD or LOGE, so I wonder if there is any other log file that I am missing? recording... should be logged out at debug level somewhere, but I don't see it. Even then, I am not sure how I would export it - main.c does not seem to have a main.h, so I'm a little lost here :(

@Master92
Copy link
Contributor

Yep, recorder is a separate application that runs on its own. Therefore, it logs somewhere else, yet I also don't know where 😅
Hm - second thought is maybe add a command to the recorder that does what you intend to do and issue that from the HDZGOGGLE application just like start and stop commands. Even though it might duplicate some code 😕

@SumolX
Copy link
Contributor

SumolX commented Feb 1, 2024

@evilC perhaps mark this PR as a draft until it is ready in order to prevent an accidental merge into main before you feel it's ready.

@evilC evilC marked this pull request as draft February 1, 2024 13:56
@evilC
Copy link
Contributor Author

evilC commented Feb 1, 2024

@evilC perhaps mark this PR as a draft until it is ready in order to prevent an accidental merge into main before you feel it's ready.

Agreed. I have had to shelve this for the time being as it seems that it is a little more involved than originally thought

@qvasic
Copy link
Contributor

qvasic commented Feb 8, 2024

A couple of days ago I had an idea for similar feature so it is nice to see that somebody is already doing it. :)
I'll take a look at your code and test it.

@evilC
Copy link
Contributor Author

evilC commented Feb 8, 2024

A couple of days ago I had an idea for similar feature so it is nice to see that somebody is already doing it. :) I'll take a look at your code and test it.

The existing code will not work. My initial code did, but called walk_sdcard, which is probably a bad idea.
I need to re-visit it at some point, but have been a bit busy lately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Long press of Func button breaks recording and marks as Hot Clip
4 participants